-
-
Notifications
You must be signed in to change notification settings - Fork 237
Quake 3 demos: Handle corrupted files more gracefully #414
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
I am not exactly sure, but it looks like it is possible that files that were corrupted may appear as if they were healthy. From what I can see, in a healthy demo file all messages except the final demo message have non empty message data. With the proposed changes, when a corrupted file is encountered, the last message will still exist and have both In that case here is a possible way to skip the entire last message, including the message number and its length so the last message will have valid data indicating the existence of a problem. Instead of adding the code: bool corrupted = false;
if(len+$ > std::mem::size()){
corrupted = true;
}
if(!corrupted && (len != FINAL_DEMO_MESSAGE_LENGTH || messageNum != FINAL_DEMO_MESSAGE_NUMBER)) { You can just add the code: if(len+$ > std::mem::size()){
continue;
} leaving the rest of the original code as it was.Continue will effectively delete all the data read so far for the current array element and attempt to process another element again but in this case eof would have been reached so the array will just stop. It is very possible that I am not interpreting things correctly and the proposed changes make no sense in which case I apologize. |
Well, naturally there is many different ways a file can get corrupted. The situation I am trying to handle is that the final message has data, but the data is incomplete, aka the message is partially written. My first attempt was to have this: if(len+$ <= std::mem::size() && (len != FINAL_DEMO_MESSAGE_LENGTH || messageNum != FINAL_DEMO_MESSAGE_NUMBER)) { But that led to an exception too because it just kept reading the message contents as if they were new message headers until it eventually crashed and burned. That's why I added the corrupted flag and am also checking it at the bottom alongside the EOF condition. Maybe I'm misunderstanding your suggestion, but once this kind of corruption is encountered, there isn't much that can be done I think except abort parsing the rest of the demo. A way to improve might be to still read the data of the final message but have it marked as incomplete/corrupted in the overview so that it doesn't leave an unparsed piece at the end and the user can see that corruption exists there. The current way just prevents it from throwing an exception. |
It is true that Under normal circumstances after the message number and the value of So in view of the fact that when message corruption occurs the data may be truncated and not always empty, the following amended code should work instead of the code proposed before. if(len+$ > std::mem::size()){
u8 bytesLeft[std::mem::size()-$];
continue;
} This will skip all the data left and terminate the array leaving only patterns that contain whole messages and the corrupted message will be completely skipped. The important thing to remember is that Of course this only addresses the case of the game crashing while writing the demo. Other forms of corruption would have to be addressed independently. |
If the game crashes while writing a demo, it can end up cut off at the end. Added a small safeguard against it so that that final corrupted message will simply be ignored.